impr(polyglot): Add new structure (wordsMap) to PolyglotWordset to get words from any language by uniform distribution (@IliyaZinoviev)#7440
Open
IliyaZinoviev wants to merge 1 commit intomonkeytypegame:masterfrom
Conversation
IliyaZinoviev
changed the title
IliyaZinoviev
force-pushed
the
feature/balanced-polyglot-v3
branch
2 times, most recently
from
2d5e9ad to
ed49d47
Compare
…t words from any language by uniform distribution (@IliyaZinoviev)
IliyaZinoviev
force-pushed
the
feature/balanced-polyglot-v3
branch
from
ed49d47 to
52be7ba
Compare
github-actions
bot
added
the
waiting for review
Contributor
There was a problem hiding this comment.
Pull request overview
Adjusts polyglot word generation so languages are sampled (approximately) uniformly, rather than being dominated by languages with larger dictionaries.
Changes:
- Refactors
PolyglotWordsetto store per-languageWordsets (wordsMap) and pick a language uniformly per generated word. - Updates word generation logic to use
PolyglotWordset.currentLanguageinstead of per-word language lookups. - Small control-flow cleanup in word list retrieval / funbox wordset assignment.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/ts/test/words-generator.ts | Switches polyglot language detection to currentLanguage; minor cleanup around word list returns and funbox wordset assignment. |
| frontend/src/ts/test/funbox/funbox-functions.ts | Reworks PolyglotWordset internals to use wordsMap and uniform language selection. |
Comment on lines
+158
to
+166
| super([]); | ||
| this.languageProperties = languageProperties; | ||
| this.langs = Array.from(languageProperties.keys()); | ||
| this.wordsMap = wordsMap; | ||
| this.resetIndexes(); | ||
| this.length = Array.from(this.wordsMap.values()).reduce( | ||
| (sum, ws) => sum + ws.words.length, | ||
| 0, | ||
| ); |
There was a problem hiding this comment.
PolyglotWordset now calls super([]) and never populates this.words. Code in words-generator relies on currentWordset.words (eg repeat-quote path + hasTab/hasNewline checks), so polyglot wordsets will report an empty list. Consider keeping words as an aggregate of all per-language words (or overriding/adding an accessor and updating consumers to use it).
Suggested change
| super([]); | |
| this.languageProperties = languageProperties; | |
| this.langs = Array.from(languageProperties.keys()); | |
| this.wordsMap = wordsMap; | |
| this.resetIndexes(); | |
| this.length = Array.from(this.wordsMap.values()).reduce( | |
| (sum, ws) => sum + ws.words.length, | |
| 0, | |
| ); | |
| const aggregatedWords = Array.from(wordsMap.values()).reduce<string[]>( | |
| (all, ws) => all.concat(ws.words), | |
| [], | |
| ); | |
| super(aggregatedWords); | |
| this.languageProperties = languageProperties; | |
| this.langs = Array.from(languageProperties.keys()); | |
| this.wordsMap = wordsMap; | |
| this.resetIndexes(); | |
| this.length = aggregatedWords.length; |
| } | ||
|
|
||
| override resetIndexes(): void { | ||
| this.wordsMap.forEach((ws, _) => { |
There was a problem hiding this comment.
resetIndexes() uses this.wordsMap.forEach((ws, _) => ...) but the key param is unused. Drop the second arg to avoid unused-var lint noise.
Suggested change
| this.wordsMap.forEach((ws, _) => { | |
| this.wordsMap.forEach(ws => { |
Miodec
added
waiting for update
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add uniform distribution to polyglot
Motivation
During polyglot using, I mentioned that words from language with huge dictionaries are met always, but words from small one - never (at least from a user experience perspective, it looks like this). My PR solves this issue. And now each language is consumed per test as much as any other.
Description
Changed:
Checks
packages/schemas/src/languages.tsfrontend/src/ts/constants/languages.tsfrontend/static/languagespackages/schemas/src/themes.tsfrontend/src/ts/constants/themes.tsfrontend/static/themespackages/schemas/src/layouts.tsfrontend/static/layoutsfrontend/static/webfontspackages/schemas/src/fonts.tsfrontend/src/ts/constants/fonts.tsCloses #